Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #5143: Update Robolectric to 4.7 #5326

Closed
wants to merge 1 commit into from

Conversation

sandy99405
Copy link

Explanation

According to issue #5143, there was an error unexpected element <queries> found in <manifest> during the Bazel build.
That issue has been resolved in this PR.

  1. Robolectric version has been changed to 4.7 and espresso-core to 3.5.0-alpha03 in every build.gadle file

  2. In scripts/assets folder, the maven_dependencies.textproto file has been changed due to an exception.

This is the Bazel output after updating the Robolectric to 4.7 and maven_install

image

Essential Checklist

  • The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

If your PR includes UI-related changes, then:

  • Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes
  • For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see RTL guide)
  • Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
  • For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included
  • Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

Copy link

Hi! @sandy99405 Welcome to Oppia! Could you please follow the instructions here and sign the CLA Sheet to get started? You'll need to do this before we can accept your PR. Thanks!

Signed-off-by: sandy99405 <[email protected]>
@sandy99405
Copy link
Author

Hi @adhiamboperes, do you want me to create another PR or solve what's in this PR? Please confirm

@adhiamboperes
Copy link
Collaborator

Hi @adhiamboperes, do you want me to create another PR or solve what's in this PR? Please confirm

I'm still in the process of reviewing the PR. I will leave my full feedback and assign it back to you.

@sandy99405
Copy link
Author

Sure, thank you :)

Copy link
Collaborator

@adhiamboperes adhiamboperes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sandy99405!

To address the comments that I have left, please create a new PR. Make sure your branch name is not called "develop", because it is prohibited. Read the PR essential checklist carefully.

We have a wiki page for reading the CI results to help you debug: https://github.com/oppia/oppia-android/wiki/Interpreting-CI-Results

And finally to run the bazel tests locally, follow the instructions here: https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions. Please note the you can also run one test class at a time.

'androidx.test.espresso:espresso-intents:3.1.0',
'androidx.test.ext:junit:1.1.1',
'androidx.test.ext:truth:1.4.0',
'androidx.work:work-testing:2.4.0',
'com.github.bumptech.glide:mocks:4.11.0',
'com.google.truth:truth:1.1.3',
'com.google.truth.extensions:truth-liteproto-extension:1.1.3',
'org.robolectric:annotations:4.5',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the CI logs, you will see that building with gradle fails with:

Caused by: java.lang.RuntimeException: Failed to transform '/root/.gradle/caches/modules-2/files-2.1/org.bouncycastle/bcprov-jdk15on/1.68/46a080368d38b428d237a59458f9bc915222894d/bcprov-jdk15on-1.68.jar' using Jetifier. Reason: Unsupported class file major version 59. (Run with --stacktrace for more details.)

Please refer to the following for an explanation and a fix: issue with gradle plugin, jetifier and robolectric, this comment and this other one.

Comment on lines +115 to +116
"org.robolectric:annotations": "4.7",
"org.robolectric:robolectric": "4.7",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the bazel test failure logs, I could still see that it fails with the error mentioned in the issue.

Could I propose that you test this PR by running some tests in bazel and see if it is reproducible? If say the tests always pass on your machine, we could look into the platform.

From the bazel logs, I saw that the issue is similar to this one: bazelbuild/rules_android#39, and one of the test runs fails because of the DrawableBindingAdapters.kt. You will need to verify from the CI logs if this particular class is the cause of all the test failures, and figure out why.

Copy link

oppiabot bot commented Feb 15, 2024

Hi @sandy99405, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Feb 15, 2024
@oppiabot oppiabot bot closed this Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Corresponds to items that haven't seen a recent update and may be automatically closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants